AO3-7271 Add cloudflare bot score and fingerprints to akismet comment spam checks #5567
Conversation
|
I couldn't figure out how to add response headers in tests, the current one just compares nil to nil. How can I properly test this? |
marcus8448
left a comment
marcus8448
left a comment
There was a problem hiding this comment.
I think a controller test should work (see comments/default_rails_actions_spec.rb)
| @comment.cloudflare_bot_score = headers["cf-bot-score"] | ||
| @comment.cloudflare_ja3_hash = headers["cf-ja3-hash"] | ||
| @comment.cloudflare_ja4 = headers["cf-ja4"] |
There was a problem hiding this comment.
This should be request.headers (headers is for response headers).
There was a problem hiding this comment.
Huh, Jira ticket says "available in the response header" though?
There was a problem hiding this comment.
I've tried writing a test there too, the problem was even when I added cf info to response.headers it was resetting back to default headers when creating the comment and setting akismet_attributes from there.
There was a problem hiding this comment.
Huh, Jira ticket says "available in the response header" though?
I think that might be a mistake in the ticket.
I've tried writing a test there too, the problem was even when I added cf info to
response.headersit was resetting back to default headers when creating the comment and settingakismet_attributesfrom there.
I'm fairly certain setting request.headers should work? But the actual testing is a bit more complicated since the cloudflare attributes are short lived on the Comment. I'm not sure what the best practices are here so I'll open this up for someone else to review.
There was a problem hiding this comment.
So what we use else where is req.env["HTTP_X_UNICORNS"]
Like https://github.com/otwcode/otwarchive/blob/master/config/initializers/rack_attack.rb#L48
It's what we get in the request ( the information is inserted by cloudflare )
marcus8448
added
Reviewed: Action Needed
Awaiting Review
and removed
Awaiting Review
Reviewed: Action Needed
labels
github-actions
bot
added
Coder Has Actioned Review
and removed
Reviewed: Action Needed
labels
3 participants
Pull Request Checklist
as the first thing in your pull request title (e.g.
AO3-1234 Fix thing)until they are reviewed and merged before creating new pull requests.
Issue
https://otwarchive.atlassian.net/browse/AO3-7271
Purpose
When the instance uses Cloudflare, add Cloudflare's information - which will be available on the request headers- to akismet comment checks for better accuracy. It will not add anything otherwise.
Credit
ömer faruk (he/him)
marcus8448, for the tests